Skip to content

[STORM-2093] Fix permissions in multi-tenant, secure mode#2187

Merged
asfgit merged 1 commit into
apache:masterfrom
Ethanlm:STORM-2093
Jul 10, 2017
Merged

[STORM-2093] Fix permissions in multi-tenant, secure mode#2187
asfgit merged 1 commit into
apache:masterfrom
Ethanlm:STORM-2093

Conversation

@Ethanlm

@Ethanlm Ethanlm commented Jul 5, 2017

Copy link
Copy Markdown
Contributor

Heap dumps created on OOM, when served through the logviewer in secure multi-tenant configurations have permissions set such that the logviewer cannot read them.

This change checks permissions in this case before serving and then runs the worker-launcher to enable the logviewer to serve them.

see #2032

@revans2 revans2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 looks good to me

@Ethanlm I assume that you have tested this manually?

@Ethanlm

Ethanlm commented Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

@revans2 Yes, I tested it manually.

(resp/status 404)))))

(defnk set-log-file-permissions [fname root-dir]
(let [file (.getCanonicalFile (File. root-dir fname))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation on this function is off.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will fix it quickly

parent (.getParent (File. root-dir fname))
md-file (if (nil? parent) nil (get-metadata-file-for-wroker-logdir parent))
topo-owner (if (nil? md-file) nil (get-topo-owner-from-metadata-file (.getCanonicalPath md-file)))]
(if (and run-as-user

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: since there is no 'else' branch, replacing if with when will allow you to remove the do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will fix it quickly

@knusbaum

knusbaum commented Jul 6, 2017

Copy link
Copy Markdown
Contributor

+1

@HeartSaVioR

Copy link
Copy Markdown
Contributor

+1
@Ethanlm Could you squash commits into one? Thanks in advance.

@asfgit asfgit merged commit 0201ae7 into apache:master Jul 10, 2017
@Ethanlm Ethanlm deleted the STORM-2093 branch July 20, 2017 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants